Skip to content

Conversation

amartya4256
Copy link
Contributor

@amartya4256 amartya4256 commented Sep 1, 2025

This PR adapts the retrieval of width of Caret SystemParametersInfo to be scaled as per the natie zoom as SystemParametersInfo retrieves value for the primary zoom at startup.

Copy link
Contributor

github-actions bot commented Sep 1, 2025

Test Results

   546 files  + 7     546 suites  +7   31m 18s ⏱️ - 9m 1s
 4 426 tests +54   4 409 ✅ +71   17 💤 +3  0 ❌  - 7 
16 750 runs  +54  16 623 ✅ +71  127 💤 +3  0 ❌  - 7 

Results for commit 3f54b85. ± Comparison against base commit 1bad9a2.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change currently introduces a regression:
I tested the change and printed out the old and new values for comparison. Turns out that the new method call always writes 0 to the passed buffer and, in particular, the method call always returns false, such that the size is not taken into account.
One example consequence is that if I set a high OS cursor size (like 10), the cursor is still 1 pixel wide. For example, in a console window it used to look like this (with OS cursor size 10):
image
And now it looks like this:
image

Note that the documentation of the used method states that it cannot be called with SPI_GETCARETWIDTH as done here but will always return 0 in that case: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-systemparametersinfofordpi#remarks

@amartya4256 amartya4256 force-pushed the amartya4256/use_new_api_in_caret branch from 38cac4f to ac7d9d5 Compare September 3, 2025 09:47
@amartya4256
Copy link
Contributor Author

@HeikoKlare Turns out SystemParametersInfo doesn't work with SPI_GETCARETWIDTH : https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-systemparametersinfofordpi
So, the other way of having DPI specific value is by scaling it ourselves. Hence, I have readapted this PR. Please have a look. #2470 has the final implementation with the refactored handler. Hence, this PR only acts as an intermediate state. The goal should be to make sure this doesn't cause any regression.

@amartya4256 amartya4256 changed the title Use SystemParametersInfoForDpi in Caret Scale width of Caret w.r.t. wthe native zoom Sep 3, 2025
@amartya4256 amartya4256 changed the title Scale width of Caret w.r.t. wthe native zoom Scale width of Caret w.r.t. the native zoom Sep 3, 2025
Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal should be to make sure this doesn't cause any regression.

If I understand correctly, there was actually a bug that this PR now resolves: before, the cursor width was not scaled according to the monitor. So, e.g., a Windows setting of cursor size 10 would have resulted in pixel-width 10 for the cursor on both a 100% and 150% monitor whereas now on the 150% monitor the width will be 15 pixels.
So would be good to have that properly documented in the commit and PR messages.

@amartya4256 amartya4256 force-pushed the amartya4256/use_new_api_in_caret branch from ac7d9d5 to 476a5bd Compare September 3, 2025 11:34
This commit adapts the retrieval of width of Caret SystemParametersInfo
to be scaled as per the native zoom of the widget as SystemParametersInfo retrieves
value for the primary zoom at startup.
@amartya4256 amartya4256 force-pushed the amartya4256/use_new_api_in_caret branch from 476a5bd to 3f54b85 Compare September 3, 2025 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use SystemParametersInfoForDpi in Caret
2 participants